Skip to content

#735 Prevent modification of the user root folder. - #736

Merged
adiroiban merged 6 commits into
masterfrom
735-prevent-virtual-root-delete
Jul 29, 2026
Merged

#735 Prevent modification of the user root folder.#736
adiroiban merged 6 commits into
masterfrom
735-prevent-virtual-root-delete

Conversation

@adiroiban

@adiroiban adiroiban commented Jul 22, 2026

Copy link
Copy Markdown
Member

Scope

Fixes #735

Update the filesystem handler to prevent operation on the root folder.

Changes

Add a helper to raise an exception.
Guard all operations that can mutate the root.

How to try and test the changes

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@adiroiban adiroiban changed the title Initial implementation. #735 Prevent modification of the user root folder. Jul 22, 2026
@greptile-apps

greptile-apps Bot commented Jul 25, 2026

Copy link
Copy Markdown
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/chevah_compat/unix_filesystem.py:222-224
**Unreachable legacy root check**

The `if path == '/'` guard (line 224) is now dead code. `_rejectRoot` at the top of this method calls `isRoot`, which already raises `CompatError(1009, ...)` before execution reaches the old check. The two checks also emit different error messages, which could be confusing during debugging if the legacy block were ever reachable.

### Issue 2 of 2
src/chevah_compat/posix_filesystem.py:390-396
**Case-insensitive comparison applied to POSIX paths**

`isRoot` uses `.lower()` on both paths unconditionally. On case-sensitive POSIX filesystems (Linux ext4, etc.), a virtual root configured as `/Home/sftp_root` and a path `/home/sftp_root` would be considered equal, incorrectly blocking operations on the distinct lowercase path. Consider limiting the case-insensitive fold to Windows only, or using `os.path.normcase` which already applies `.lower()` on Windows and is a no-op on POSIX.

```suggestion
        return os.path.normcase(root) == os.path.normcase(path)
```

Reviews (1): Last reviewed commit: "Initial implementation." | Re-trigger Greptile

Comment thread src/chevah_compat/unix_filesystem.py Outdated
Comment thread src/chevah_compat/posix_filesystem.py
@greptile-apps

greptile-apps Bot commented Jul 27, 2026

Copy link
Copy Markdown

Reviews (2): Last reviewed commit: "Update for latest ruff." | Re-trigger Greptile

Comment thread src/chevah_compat/tests/normal/test_filesystem.py
@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown
Prompt To Fix All With AI
### Issue 1
src/chevah_compat/tests/normal/test_filesystem.py:34-42
**NT unlocked filesystem: `['..']` and `['child', '..']` raise `OSError`, not `CompatError`**

On the NT unlocked filesystem (`lock_in_home_folder = False`), `getRealPathFromSegments` interprets the first segment as a drive letter. Passing `['..']` produces `drive = '..\\'`, which fails `_validateDrivePath` with `OSError(EINVAL)` — well before `_rejectRoot` can raise `CompatError(1009)`. The same happens for `['child', '..']` because `'child'` is not a single-letter drive. `assertRaises(CompatError, …)` does not catch `OSError`, so `test_deleteFolder_root` in `TestLocalFilesystemUnlocked` will fail on Windows for those two segment forms. The locked filesystem is not affected because `_getLockedPathFromSegments` collapses `['..']` back to `root_path` before any drive validation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (3): Last reviewed commit: "Update tests with Claude." | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown

Reviews (4): Last reviewed commit: "Fix isRoot." | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown

Want your agent to iterate on Greptile's feedback? Try greploops.

@gitar-bot

gitar-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Adds filesystem root guards to prevent mutating the user root folder alongside a version bump to 1.7.0 and ruff updates. No issues found.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown

Reviews (5): Last reviewed commit: "Fix for nt root drive." | Re-trigger Greptile

@adiroiban
adiroiban merged commit e5c164f into master Jul 29, 2026
17 checks passed
@adiroiban
adiroiban deleted the 735-prevent-virtual-root-delete branch July 29, 2026 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't delete the root segments

1 participant